chore(ci): Add dev tests with master dependencies #55

kramaranya · 2025-08-01T07:58:20Z

What this PR does / why we need it:
I added a test-python-dev target and changed the unit test CI jobs with a dev/prod matrix

Dev leg installs the dev dependency group, which now pulls kubeflow_trainer_api from the master branch. Prod leg will run with the packages published to PyPI (once API models are available there)

I also updated the CI job to run unit tests for two Python vesions -- 3.9 and 3.11

I removed flake8 since ruff replaced it

/assign @andreyvelich @astefanutti @Electronic-Waste @tenzen-y @briangallagher

Checklist:

Docs included if any changes are user facing

google-oss-prow · 2025-08-01T07:58:24Z

@kramaranya: GitHub didn't allow me to assign the following users: briangallagher.

Note that only kubeflow members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

What this PR does / why we need it:
I added a test-python-dev target and changed the unit test CI jobs into a dev/prod matrix

Dev leg installs the dev dependency group, which now pulls kubeflow_trainer_api from the master branch. Prod leg will run with the packages published to PyPI (once API models are available there)

/assign @andreyvelich @astefanutti @Electronic-Waste @tenzen-y @briangallagher

Checklist:

Docs included if any changes are user facing

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

coveralls · 2025-08-01T08:01:32Z

Pull Request Test Coverage Report for Build 16750375247

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 66.038%

Totals
Change from base Build 16727070395:	0.0%
Covered Lines:	280
Relevant Lines:	424

💛 - Coveralls

Electronic-Waste

@kramaranya Thanks for this!
/lgtm

astefanutti · 2025-08-01T13:33:24Z

/lgtm

That's great, thanks!

kramaranya · 2025-08-03T23:35:40Z

@astefanutti @Electronic-Waste thanks for your review!
I’ve updated the CI to run unit tests on Python 3.9 and 3.11, and removed flake8 because ruff replaced it in #38
Could you please take a look again?

andreyvelich

Thank you for this @kramaranya!

andreyvelich · 2025-08-04T00:43:59Z

python/pyproject.toml

 [dependency-groups]
 dev = [
    "ruff>=0.12.2",
+    "kubeflow_trainer_api@git+https://github.com/kubeflow/trainer.git@master#subdirectory=api/python_api",


Is there any use-case when users would like to install Kubeflow SDK with the latest module package ?
IIUC, the dependency-groups is never published to PyPI by hatch, so users can't install.
Thoughts @astefanutti @szaher @briangallagher @kramaranya @eoinfennessy ?

Yeah, I've been thinking about adding this to project.optional-dependencies as well, but since we don't update API models that often, it makes sense to me to keep models from master available only for testing. However, I'm open to other suggestions if you see it differently

From my point of view, since we allow to install control plane with the latest changes, users might want to experiment with it during the development: https://www.kubeflow.org/docs/components/trainer/operator-guides/installation/#installing-the-kubeflow-trainer-controller-manager

This gives opportunities for them to quickly adopt newer API changes into their systems.

Makes sense, let me update it

Updated in 4d8967f

andreyvelich · 2025-08-04T00:46:36Z

Makefile

+.PHONY: test-python-dev
+test-python-dev: uv-venv
+	@uv pip install --project $(PY_DIR) --group dev "$(PY_DIR)[test]"
+	@uv run coverage run --source=kubeflow.trainer.api.trainer_client,kubeflow.trainer.utils.utils -m pytest ./python/kubeflow/trainer/api/trainer_client_test.py
+	@uv run coverage report -m kubeflow/trainer/api/trainer_client.py kubeflow/trainer/utils/utils.py
+ifeq ($(report),xml)
+	@uv run coverage xml
+else
+	@uv run coverage html
+endif


Can we run tests over the master modules for now ?
Since we can't introduce any breaking changes to the Kubernetes API, I don't see a need to run tests against released modules.

Since we can't introduce any breaking changes to the Kubernetes API

Hmm, why can't we?

According to Kubernetes API best practices, if we introduce any breaking changes to the existing API: v1alpha1, we have to create the newer API version, for instance: v1alpha2: https://kubernetes.io/docs/reference/using-api/deprecation-policy/

cc @tenzen-y @astefanutti

In the future, we should discuss how to design our SDK to make it compatible with supported API versions, but we are not there yet.

Thanks a lot for explanation! That makes sense now to test it only against master branch
Updated in 4d8967f

andreyvelich · 2025-08-04T00:47:12Z

.github/workflows/test-python.yaml

+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true


IIRC, we discussed before that this part is not needed given that we don't have limitations on concurrent runs due to GitHub enterprise.

A use case is re-pushing the branch for the PR while the workflow is running. Even with no limitations, is that useful to keep the existing run going?

Might be useful to compare results, but I am fine to cancel them for now.
@kramaranya Please can you apply the change to all GitHub action tests in that case ?

Sure, updated in 4853da4

andreyvelich · 2025-08-04T00:48:11Z

.github/workflows/test-python.yaml

+          flag-name: ${{ matrix.dependency-group.name }}-py${{ matrix.python-version }}
+          parallel: true
+
+  coverage:


Don't we run coverage as part of the previous job in Upload coverage to Coveralls step ?

As I can see, it shows the report on the PRs: #56 (comment)

Yeah, I added the parallel + “coverage-finished” step there when the matrix had four legs (dev/prod × two Python versions), so Coveralls could wait for every flag upload before closing the build and merging reports -- https://docs.coveralls.io/parallel-builds

But I've removed it in 189139d since we're now testing against the master modules only

kramaranya · 2025-08-04T18:50:13Z

/test all

google-oss-prow · 2025-08-04T18:50:17Z

@kramaranya: No presubmit jobs available for kubeflow/sdk@main

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: kramaranya <kramaranya15@gmail.com>

andreyvelich · 2025-08-04T20:10:53Z

python/pyproject.toml

Let's also migrate to the official package as part of this PR: https://pypi.org/project/kubeflow-trainer-api/

Signed-off-by: kramaranya <kramaranya15@gmail.com>

kramaranya · 2025-08-05T00:36:14Z

@andreyvelich @astefanutti thank you for the review!
I should have addressed all your comments and imported kubeflow-trainer-api from PyPI

sdk/python/pyproject.toml

Line 31 in e84a54f

"kubeflow-trainer-api>=2.0.0",

.
Please let me know if there's anything else I should update!

astefanutti · 2025-08-05T07:41:30Z

/lgtm

Thanks @kramaranya!

andreyvelich · 2025-08-05T10:31:54Z

CONTRIBUTING.md

+
 Install development tools:
 ```sh
 pip install pytest black isort flake8 coverage pre-commit


I guess, we can remove it since we have everything in dev, right ?

yep, updated in cb1c4df

andreyvelich · 2025-08-05T10:33:36Z

Makefile

 .PHONY: test-python
 test-python: uv-venv
-	@uv pip install "./python[test]"
+	@uv pip install --project $(PY_DIR) --group dev "$(PY_DIR)[test,dev]"


Please can we move ruff and test packages to dev too ?

In that case we have all tools required for development in one place

Sure, shall we also add pre-commit package to dev?

great, updated in cb1c4df!

Signed-off-by: kramaranya <kramaranya15@gmail.com>

kramaranya · 2025-08-05T11:24:22Z

@andreyvelich thank you for the review! Let me please know if there's anything else I should update!

andreyvelich · 2025-08-05T12:04:33Z

Makefile

 .PHONY: test-python
 test-python: uv-venv
-	@uv pip install "./python[test]"
+	@uv pip install --project $(PY_DIR) -e "$(PY_DIR)[dev]"


Do we need this --project flag ?

I added it for project isolation, but since we have only one project right now, I can remove it

done in 542ddcd

Signed-off-by: kramaranya <kramaranya15@gmail.com>

andreyvelich

Thanks for this @kramaranya!
/lgtm
/approve

google-oss-prow · 2025-08-05T12:48:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [andreyvelich]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

google-oss-prow bot assigned andreyvelich, astefanutti, Electronic-Waste and tenzen-y Aug 1, 2025

google-oss-prow bot requested review from Electronic-Waste, andreyvelich and astefanutti August 1, 2025 07:58

google-oss-prow bot added the size/M label Aug 1, 2025

Electronic-Waste reviewed Aug 1, 2025

View reviewed changes

google-oss-prow bot added the lgtm label Aug 1, 2025

google-oss-prow bot removed the lgtm label Aug 3, 2025

kramaranya force-pushed the dev-testing branch from 2f305de to a06cd40 Compare August 3, 2025 22:53

andreyvelich reviewed Aug 4, 2025

View reviewed changes

kramaranya added 6 commits August 4, 2025 20:20

add make target test-python-dev

935f2b7

Signed-off-by: kramaranya <kramaranya15@gmail.com>

add kubeflow_trainer_api master to dev deps

b9c3dd7

Signed-off-by: kramaranya <kramaranya15@gmail.com>

split unit test CI job into dev/prod matrix

e354215

Signed-off-by: kramaranya <kramaranya15@gmail.com>

update testing info in CONTRIBUTING.md

2b5df31

Signed-off-by: kramaranya <kramaranya15@gmail.com>

run CI jobs with different Python versions

bf65a78

Signed-off-by: kramaranya <kramaranya15@gmail.com>

remove flake8

ef4ebfc

Signed-off-by: kramaranya <kramaranya15@gmail.com>

kramaranya force-pushed the dev-testing branch from f3377b4 to ef4ebfc Compare August 4, 2025 19:20

andreyvelich reviewed Aug 4, 2025

View reviewed changes

kramaranya added 3 commits August 5, 2025 00:29

import kubeflow-trainer-api from PyPI

aa92352

Signed-off-by: kramaranya <kramaranya15@gmail.com>

run unit tests against master modules only

4d8967f

Signed-off-by: kramaranya <kramaranya15@gmail.com>

remove parallel run of CI

189139d

Signed-off-by: kramaranya <kramaranya15@gmail.com>

kramaranya added 2 commits August 5, 2025 01:15

add cancel-in-progress to CI jobs

4853da4

Signed-off-by: kramaranya <kramaranya15@gmail.com>

update CONTRIBUTING.md

e84a54f

Signed-off-by: kramaranya <kramaranya15@gmail.com>

google-oss-prow bot added the lgtm label Aug 5, 2025

andreyvelich reviewed Aug 5, 2025

View reviewed changes

move dev tools to [dev] deps

cb1c4df

Signed-off-by: kramaranya <kramaranya15@gmail.com>

google-oss-prow bot added size/L and removed lgtm size/M labels Aug 5, 2025

andreyvelich reviewed Aug 5, 2025

View reviewed changes

remove uv --project flag

542ddcd

Signed-off-by: kramaranya <kramaranya15@gmail.com>

andreyvelich reviewed Aug 5, 2025

View reviewed changes

google-oss-prow bot added the lgtm label Aug 5, 2025

google-oss-prow bot added the approved label Aug 5, 2025

google-oss-prow bot merged commit 7614d74 into kubeflow:main Aug 5, 2025
9 checks passed

google-oss-prow bot added this to the v0.1 milestone Aug 5, 2025

kramaranya deleted the dev-testing branch August 8, 2025 16:53

andreyvelich mentioned this pull request Aug 24, 2025

PyPI Publishing Blocked by Git Dependencies #70

Closed

chore(ci): Add dev tests with master dependencies #55

chore(ci): Add dev tests with master dependencies #55

Conversation

kramaranya commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-prow bot commented Aug 1, 2025

Uh oh!

coveralls commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 16750375247

Details

💛 - Coveralls

Uh oh!

Electronic-Waste left a comment

Choose a reason for hiding this comment

Uh oh!

astefanutti commented Aug 1, 2025

Uh oh!

kramaranya commented Aug 3, 2025

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreyvelich Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kramaranya commented Aug 4, 2025

Uh oh!

google-oss-prow bot commented Aug 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kramaranya commented Aug 5, 2025

Uh oh!

astefanutti commented Aug 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kramaranya commented Aug 1, 2025 •

edited

Loading

coveralls commented Aug 1, 2025 •

edited

Loading

andreyvelich Aug 4, 2025 •

edited

Loading